Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Jan 20, 2026

Initially started a couple of years ago, the new cjs module lexer is 25% faster for cold importing cjs files. This change introduces a new dependency and removes the old one.

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1784/console

                                                                                                                                                                confidence improvement accuracy (*)
esm/detect-esm-syntax.js n=10000 type='with-package-json'                                                                                                              ***      1.47 %       ±0.40%
esm/detect-esm-syntax.js n=10000 type='without-package-json'                                                                                                           ***      0.64 %       ±0.35%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/non-exist' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000                *      0.95 %       ±0.92%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/non-exist' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000                                -0.06 %       ±0.95%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.js' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000                  -0.72 %       ±1.02%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.js' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000                             0.33 %       ±0.62%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.json' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000                 0.38 %       ±0.98%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.json' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000                   **      0.84 %       ±0.61%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.node' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000                 0.66 %       ±1.04%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.node' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000                           0.21 %       ±0.92%
esm/esm-loader-defaultResolve.js specifier='./relative-existing.js' n=1000                                                                                                     -1.93 %       ±5.48%
esm/esm-loader-defaultResolve.js specifier='./relative-nonexistent.js' n=1000                                                                                                  -0.48 %       ±1.94%
esm/esm-loader-defaultResolve.js specifier='node:os' n=1000                                                                                                                    -8.43 %       ±9.28%
esm/esm-loader-defaultResolve.js specifier='node:prefixed-nonexistent' n=1000                                                                                                  -8.48 %      ±10.15%
esm/esm-loader-defaultResolve.js specifier='unprefixed-existing' n=1000                                                                                                         0.56 %       ±3.21%
esm/esm-loader-defaultResolve.js specifier='unprefixed-nonexistent' n=1000                                                                                                     -0.08 %       ±1.73%
esm/esm-loader-import.js specifier='./relative-existing.js' n=1000                                                                                                       *      5.62 %       ±5.09%
esm/esm-loader-import.js specifier='./relative-nonexistent.js' n=1000                                                                                                           1.53 %       ±2.90%
esm/esm-loader-import.js specifier='data:text/javascript,{i}' n=1000                                                                                                            1.47 %       ±2.73%
esm/esm-loader-import.js specifier='node:os' n=1000                                                                                                                             0.43 %       ±7.59%
esm/esm-loader-import.js specifier='node:prefixed-nonexistent' n=1000                                                                                                           2.29 %       ±3.67%
esm/import-cjs.js type='cold'                                                                                                                                          ***     26.83 %       ±0.58%
esm/import-cjs.js type='warm'                                                                                                                                          ***     11.95 %       ±0.14%
esm/import-esm-reload.js n=1000 count=1                                                                                                                                         0.08 %       ±1.04%
esm/import-esm-reload.js n=1000 count=100                                                                                                                                *      0.99 %       ±0.92%
esm/import-meta.js valuesToRead='dirname-and-filename' n=1000                                                                                                           **      1.11 %       ±0.75%
esm/import-meta.js valuesToRead='dirname' n=1000                                                                                                                                0.50 %       ±0.67%
esm/import-meta.js valuesToRead='filename' n=1000                                                                                                                       **      1.17 %       ±0.76%
esm/import-meta.js valuesToRead='url' n=1000                                                                                                                                   -0.57 %       ±0.77%
esm/require-esm.js n=1000 exports='default' type='access'                                                                                                                       0.77 %       ±1.21%
esm/require-esm.js n=1000 exports='default' type='all'                                                                                                                          1.31 %       ±1.75%
esm/require-esm.js n=1000 exports='default' type='load'                                                                                                                         0.06 %       ±1.96%
esm/require-esm.js n=1000 exports='named' type='access'                                                                                                                         0.79 %       ±1.05%
esm/require-esm.js n=1000 exports='named' type='all'                                                                                                                           -0.54 %       ±1.98%
esm/require-esm.js n=1000 exports='named' type='load'                                                                                                                          -0.90 %       ±2.32%

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Jan 20, 2026
@anonrig anonrig force-pushed the yagiz/experiment-with-cjs-lexer branch 3 times, most recently from ae6ab98 to 5e69495 Compare January 21, 2026 16:24
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 78.84615% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.78%. Comparing base (98d2331) to head (474c7e1).
⚠️ Report is 53 commits behind head on main.

Files with missing lines Patch % Lines
src/node_cjs_lexer.cc 77.55% 8 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61456      +/-   ##
==========================================
- Coverage   89.85%   89.78%   -0.07%     
==========================================
  Files         671      673       +2     
  Lines      203166   203822     +656     
  Branches    39057    39183     +126     
==========================================
+ Hits       182554   183001     +447     
- Misses      12967    13139     +172     
- Partials     7645     7682      +37     
Files with missing lines Coverage Δ
lib/internal/modules/esm/translators.js 92.58% <100.00%> (-0.39%) ⬇️
src/node_binding.cc 82.74% <ø> (ø)
src/node_builtins.cc 76.33% <ø> (+0.14%) ⬆️
src/node_metadata.cc 92.75% <100.00%> (+0.32%) ⬆️
src/node_cjs_lexer.cc 77.55% <77.55%> (ø)

... and 72 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonrig anonrig force-pushed the yagiz/experiment-with-cjs-lexer branch 13 times, most recently from 9bfb358 to 1da16dd Compare January 21, 2026 19:44
@anonrig anonrig changed the title [DO NOT MERGE] experiment with commonjs-lexer build,deps: replace cjs-module-lexer with merve Jan 21, 2026
@anonrig anonrig requested a review from guybedford January 21, 2026 19:46
@anonrig anonrig force-pushed the yagiz/experiment-with-cjs-lexer branch 5 times, most recently from 66e99b4 to d687c0a Compare January 21, 2026 20:29
@anonrig anonrig requested a review from mcollina January 21, 2026 20:56
@mcollina
Copy link
Member

Where does this new dep live?

@anonrig
Copy link
Member Author

anonrig commented Jan 22, 2026

Where does this new dep live?

Currently, under my github account. Is anybody interested in helping me maintain it? github.com/anonrig/merve

@anonrig anonrig force-pushed the yagiz/experiment-with-cjs-lexer branch from d687c0a to b2ec704 Compare January 22, 2026 15:24
@guybedford
Copy link
Contributor

Per current process it would make sense to move merve into the Node.js org as well. I can then add a note to the cjs-module-lexer readme that future bugs should be posted there. I'd value also having permissions to the new repo to continue to provide support.

Copy link
Member

@lemire lemire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next step, name a library after your cat.

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update these references to the removed cjs-module-lexer:

@anonrig anonrig requested a review from richardlau January 26, 2026 19:14
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #61456 (comment), I would prefer the dependency was in the Node.js org (as the dep it is replacing, cjs-module-lexer, is).

@anonrig
Copy link
Member Author

anonrig commented Jan 27, 2026

cc @nodejs/tsc are we all ok with moving https://github.com/anonrig/merve to node.js organization? my only ask is to keep the name and don't change it in the future.

@aduh95
Copy link
Contributor

aduh95 commented Jan 27, 2026

cc @nodejs/tsc are we all ok with moving https://github.com/anonrig/merve to node.js organization? my only ask is to keep the name and don't change it in the future.

Yeah, just follow the process described in https://github.com/nodejs/admin/blob/main/transfer-repo-into-the-org.md. Happy to help if needed!

@anonrig
Copy link
Member Author

anonrig commented Jan 27, 2026

cc @nodejs/tsc are we all ok with moving https://github.com/anonrig/merve to node.js organization? my only ask is to keep the name and don't change it in the future.

Yeah, just follow the process described in https://github.com/nodejs/admin/blob/main/transfer-repo-into-the-org.md. Happy to help if needed!

Appreciate any help @aduh95, I'll give you admin rights to the repo.

@anonrig anonrig added the notable-change PRs with changes that should be highlighted in changelogs. label Jan 28, 2026
@github-actions
Copy link
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @anonrig.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@anonrig
Copy link
Member Author

anonrig commented Jan 28, 2026

let's land this pull request and i'll follow up with the changes to move the repo to nodejs organization and update references.

@anonrig anonrig added commit-queue Add this label to land a pull request using GitHub Actions. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 28, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 28, 2026
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 28, 2026
@nodejs-github-bot nodejs-github-bot merged commit 37e4004 into nodejs:main Jan 28, 2026
70 of 71 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 37e4004

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants